feat: propagate file_format_version to CommitBuilder.storageFormat() - #730
Merged
hamersaw merged 5 commits intoAug 13, 2026
Merged
Conversation
When file_format_version is set in Spark write options, lance-spark correctly encodes fragment data files in the requested format. However, at commit time the manifest's data_storage_format was never updated because CommitBuilder.storageFormat() was never called. Forward writeOptions.getFileFormatVersion() to commitBuilder.storageFormat() in all code paths that construct a CommitBuilder: - LanceBatchWrite.commit() (batch write Append/Overwrite) - StagedCommit.commitNewTable() and commitExistingTable() (staged catalog operations CREATE/REPLACE/CREATE_OR_REPLACE) - SparkPositionDeltaWrite (row-level UPDATE/DELETE/MERGE, Spark 3.4+3.5) - AddColumnsBackfillBatchWrite (add-columns backfill) - UpdateColumnsBackfillBatchWrite (column rewrite backfill) Add fileFormatVersion field to StagedCommitOptions so staged commit paths receive the value from the catalog's CreateTableSpec resolution. When fileFormatVersion is null (user didn't set the option), nothing changes — behavior is identical to before.
ivscheianu
marked this pull request as draft
July 29, 2026 04:46
hamersaw
self-requested a review
July 29, 2026 14:16
hamersaw
added a commit
to lance-format/lance
that referenced
this pull request
Aug 3, 2026
…ersion::from_str (#8063) Fixes #8066 The JNI `parse_storage_format` (used by `CommitBuilder.storageFormat()`) had a hand-rolled match that only accepted prefixed aliases (`"v2_1"`, `"v2.1"`) while `extract_write_params` uses `LanceFileVersion::from_str` which accepts the canonical numeric forms (`"2.1"`, `"2.2"`). This surfaced when [lance-spark#730](lance-format/lance-spark#730) started propagating `file_format_version` to `CommitBuilder.storageFormat()`, breaking some tests. **Fix:** - Replace the custom match with `name.parse::<LanceFileVersion>()`. - Extend `FromStr` to also accept the prefixed aliases so no previously valid input is rejected. - Update `CommitBuilder.storageFormat()` Javadoc. - Add tests for canonical forms, prefixed aliases, case-insensitivity, and invalid input. --------- Co-authored-by: Daniel Rammer <hamersaw@protonmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…age-format # Conflicts: # pom.xml
ivscheianu
marked this pull request as ready for review
August 6, 2026 07:46
Contributor
Author
|
Hello, @hamersaw! Bumped the lance-core after the latest release that include the changes needed. Please have a look when you find some time, thank you! |
![lance-gatekeeper[bot]](https://avatars.githubusercontent.com/in/4433141?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Gate recommendation: request changes.
The staged commit must use the final resolved write-time file_format_version, including DataFrameWriterV2.option(...), so empty writes preserve the same fragment/manifest contract as non-empty writes. Carry the late logical write option into StagedCommit with write-option precedence and keep null/unset behavior delegated to lance-core.
Please mark this PR with the breaking-change label.
| // The non-staged path (LanceBatchWrite) uses boxed Boolean because null means | ||
| // "user didn't specify" and lets lance-core inherit the flag from the manifest. | ||
| private boolean enableStableRowIds; | ||
| private final String fileFormatVersion; |
There was a problem hiding this comment.
This freezes the format before LogicalWriteInfo.options() is parsed, so a staged DataFrameWriterV2.option("file_format_version", ...) value never reaches the commit. With an empty write there are no fragments from which lance-core can infer the format, and the requested version is silently ignored.
Make the staged format updateable from the finalized LanceSparkWriteOptions, with the explicit write option taking precedence, and add an empty staged-writer regression.
Reproducer
testData
.limit(0)
.writeTo("lance.`" + path + "`")
.using("lance")
.option(LanceSparkWriteOptions.CONFIG_FILE_FORMAT_VERSION, "2.0")
.create();
try (org.lance.Dataset ds =
org.lance.Dataset.open().allocator(LanceRuntime.allocator()).uri(path).build()) {
assertEquals("2.0", ds.getLanceFileFormatVersion());
}Run with:
./mvnw -Djava.io.tmpdir=/home/agent/tmp/pr730-head-writerv2-jni-9BpP20 \
test -pl lance-spark-3.5_2.13 \
-Dtest=SparkConnectorWriteTest#writeWithStorageVersionOptionObserved on this head: expected: <2.0> but was: <2.1>.
lance-gatekeeper
Bot
added
the
K-changes
When DataFrameWriterV2.option("file_format_version", ...) sets the
format at write time, LanceBatchWrite.commit() now forwards it to
StagedCommit via the new setFileFormatVersion setter. Write-time
takes precedence over the stage-time value; null write-time preserves
whatever the catalog resolved at stage time.
Without this, fragments are encoded in the requested format but the
manifest's data_storage_format remains at the default, causing
check_storage_version to reject the commit.
lance-gatekeeper
Bot
removed
the
K-changes
There was a problem hiding this comment.
✅ Gate recommendation: approve.
The staged WriterV2 issue is fixed: the finalized file_format_version now updates StagedCommit with explicit write-option precedence, while an unset write option preserves the stage-time value. The empty WriterV2 create path now commits the requested manifest format.
Please mark this PR with the breaking-change label.
lance-gatekeeper
Bot
added
the
K-approved
summaryzb
added a commit
to summaryzb/lance-spark
that referenced
this pull request
Aug 14, 2026
Incorporates three upstream feats on top of the distributed vector-index work: - upstream lance-format#748 (build btree fragments with scalar segments): BTREE fragment-mode now goes through the shared ScalarSegmentIndexJob path. IndexUtils.scalarSegmentIndexTypes/leafFieldIndexTypes and resolveIndexField are pulled in; the local IVF logical-segment dispatch coexists with BTREE range-mode as a dedicated early branch. - upstream lance-format#741 (OTel for spark connector): LanceRuntime OTel bootstrapping and spark.lance.otel.enabled config auto-merged; docs and shaded whitelists updated. - upstream lance-format#730 (file_format_version -> CommitBuilder.storageFormat): write commit paths forward writeOptions.getFileFormatVersion(). Conflict resolution — AddIndexExec.scala (only true conflict): - Kept the distribute_vec_create scaffolding: baseReadOptions + extractNamespaceInfo, snapshot with pinned version and vectorPlan, train=false rejection for IVF_*, empty-table IVF rejection, and the VectorIndexJob.runSegments dispatch inside useLogicalSegmentCommit. - Adopted upstream's IndexUtils.resolveIndexField (leaf-only: BTREE/BITMAP/NGRAM/BLOOM_FILTER) and upstream's single-column error wording ("<TYPE> indexes currently support a single column only") to align with BaseAddIndexTest.testIndexesRejectMultipleColumns's substring assertion. - Added upstream's precise BTREE range-mode num_segments rejection ("num_segments is only supported for BTREE indexes with build_mode='fragment'") in front of the existing useLogicalSegmentCommit gate. - BTREE range-mode dispatches to RangeBasedBTreeIndexJob before the logical-segment fan-out. - Kept HEAD's num_segments allow-set at useLogicalSegmentCommit (scalar-segment U IVF_*) so IVF_*.runSegments continues to honour num_segments; SQ-IVF single-segment clamp preserved. - Dropped the stale upstream `AddIndexOperation` import; imported LanceField / LanceSchema for the new resolveIndexField signature. Test alignment (auto-merge left semantic gaps to the new dispatch): - IndexUtilsTest.useLogicalSegmentCommit*: moved BTREE from the "false" assertion into the "true" set; only IVF_HNSW_FLAT stays false. - BaseAddVectorIndexTest.testRejectMultipleColumns and integration-tests test_reject_multi_column_ivf_index: assert the common substring "support a single column only" (matches both old "supports" wording and the new upstream "support" wording). Verified: make lint clean, make test all green (lance-spark-3.5_2.12), local pytest against local backend (106 passed / 5 skipped / 1 xpassed). Change-Id: Ie883365da15dd8b5d7e82b8b70a9c8d7469856e6
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When
file_format_versionis set in Spark write options, lance-spark correctly encodes fragment data files in the requested format. However, at commit timeCommitBuilder.storageFormat()was never called, so the manifest'sdata_storage_formatwas never updated. This causes lance-core'scheck_storage_versionto reject the commit.Closes #729
Depends on: lance-format/lance#8063 (lance-core must accept numeric format strings like "2.1", "2.2" in
CommitBuilder.storageFormat())Changes
Forward
writeOptions.getFileFormatVersion()tocommitBuilder.storageFormat()in all code paths that construct aCommitBuilder:LanceBatchWrite.commit()— batch write (Append/Overwrite)StagedCommit.commitNewTable()andcommitExistingTable()— staged catalog operationsSparkPositionDeltaWrite(Spark 3.4 + 3.5) — row-level UPDATE/DELETE/MERGEAddColumnsBackfillBatchWrite— add-columns backfillUpdateColumnsBackfillBatchWrite— column rewrite backfillAdd
fileFormatVersionfield toStagedCommitOptionsso staged commit paths receive the value from the catalog'sCreateTableSpecresolution.Backward Compatibility
When
fileFormatVersionis null (user didn't set the option),storageFormatis not called onCommitBuilder. Behavior is identical to before.Test Plan
All 19 existing unit tests pass.
StagedCommitOptionsTestupdated to verifygetFileFormatVersion()accessor.CI Note
The full test suite requires lance-core with the
parse_storage_formatfix from lance#8063, which extends the JNI format string parser to accept numeric strings ("2.1", "2.2") in addition to the existing prefixed variants ("v2.1", "v2_1"). Without that change, any test path that exercisesCommitBuilder.storageFormat()with a numeric string will fail with "Unknown storage format".